Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting the font of a toga.Selection (Android and Winforms) #1393

Closed
wants to merge 5 commits into from

Conversation

t-arn
Copy link
Contributor

@t-arn t-arn commented Dec 22, 2021

This PR allows us to set the font of a toga.Selection on Android and Winforms

The Android implementation requires the template from this PR:
beeware/briefcase-android-gradle-template#40

PR Checklist:

  • All new features have been tested
  • [] All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@t-arn
Copy link
Contributor Author

t-arn commented Dec 22, 2021

@freakboy3742 Please review this PR, thanks!

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not wild about the approach that you've taken here. There's a lot of logic tied up in the Java side of things; this means (a) the template is increasingly Toga specific, and (b) you need to know Java (and must be able to fix a Java template) to be able to address any bug, or add any feature.

I know there are some existing shims on the existing template; however, those are either directly related to the bootstrapping process, or are the barest possible shim that can exist in Java to be extended by Python.

My long term goal is to add some metaprogramming to the templating process so that it is possible to define the existence of these shims in Python (or, at least, as part of some sort of project configuration), and have the relevant Java generated as part of the Briefcase templating process. To that end, adding complex logic on the Java side is somewhat counterproductive.

from .base import Widget, align

BeewareSpinnerAdapter = JavaClass("org/beeware/android/BeewareSpinnerAdapter")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to replicate "Beeware" in this class name; org.beeware.android.SpinnerAdapter is sufficiently unique.

Copy link
Contributor Author

@t-arn t-arn Jan 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done that because there already is a SpinnerAdapter in Android - and it is not a class, but an Interface.
Of course, technically, it would still work because we are in a different namespace, but I thought it would be less confusing if I use a new name.

How about CustomSpinnerAdapter or ExtendedSpinnerAdapter ?

Copy link
Contributor Author

@t-arn t-arn Jan 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template is not really Toga specific.
The new Class could be used by any GUI. It just provides the necessary features to control the (size of the) font - which, in my opintion, is a short-coming of the standard implementation in Android.

@t-arn
Copy link
Contributor Author

t-arn commented Jan 7, 2022

I'm not wild about the approach that you've taken here. There's a lot of logic tied up in the Java side of things; this means (a) the template is increasingly Toga specific, and (b) you need to know Java (and must be able to fix a Java template) to be able to address any bug, or add any feature.

I know there are some existing shims on the existing template; however, those are either directly related to the bootstrapping process, or are the barest possible shim that can exist in Java to be extended by Python.

My long term goal is to add some metaprogramming to the templating process so that it is possible to define the existence of these shims in Python (or, at least, as part of some sort of project configuration), and have the relevant Java generated as part of the Briefcase templating process. To that end, adding complex logic on the Java side is somewhat counterproductive.

This is currently the only approach I know to be working. If rubicon would allow sub-classing, then we might do it all in Python code.
beeware/rubicon-java#25

@freakboy3742
Copy link
Member

This is currently the only approach I know to be working. If rubicon would allow sub-classing, then we might do it all in Python code.

This is one of those cases where we could take the quick route, but that will introduce technical debt that we will need to live with over time. I'm indicating that I'm uncomfortable with the level of technical debt that the quick solution will introduce, and that the acceptable solution here starts with a change in Rubicon (and possibly Briefcase).

@freakboy3742
Copy link
Member

Closing on the basis that this specific implementation isn't headed in the right direction. There's clearly a bigger problem that we need to resolve; I want to address that underlying problem, rather than commit resources to managing a hack to solve this specific instance of the underlying problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants